-
Notifications
You must be signed in to change notification settings - Fork 98
make EDF writer handle anonymized meas_date #1479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1479 +/- ##
=======================================
Coverage 96.96% 96.96%
=======================================
Files 43 43
Lines 10100 10119 +19
=======================================
+ Hits 9793 9812 +19
Misses 307 307 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
arg, still getting mne_bids/tests/test_fileio.py::test_lock_refcount_multiprocess - AssertionError https://github.com/mne-tools/mne-bids/actions/runs/19348373149/job/55354332061?pr=1479#step:18:541 |
|
@sappelhoff there's a stdlib feature of https://github.com/mne-tools/mne-bids/actions/runs/19348373149/job/55354332026?pr=1479#step:22:376 The string is valid ISO format, this was a bug-ish in python prior to 3.11. Is this justification enough to:
? |
|
I would be happy to have the min python pushed to 3.11. It's still 11 months to go until 3.10's EOL, but we can also make our lives a tiny bit easier. |
will do
FWIW SPEC-0 would recommend already setting 3.12 as the minimum. So switching from 3.10 to 3.11 is already a bit conservative from the perspective of our ecosystem peers. |
|
OK CIs are happy Ready for review/merge |
sappelhoff
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @drammock
|
this actually needs some changes to our required checks: https://github.com/mne-tools/mne-bids/settings/branch_protection_rules/2696889
I can have a look at this soon |
|
@sappelhoff I updated the branch protection rules to replace the 3.10 jobs with the corresponding 3.11 jobs. |
|
Thank you, Dan. |


PR Description
EDF/BDF formats restrict the
startdate(equivalent of meas_date) to be 1985-01-01 or later, which conflicts with the BIDS standard for anonymization (which requires anonymized dates to be before 1920-01-01). In #669 a decision was made to write EDF(+) files that violate the BIDS standard but honor the EDF standard (making them viewable in EDFBrowser), while writing the BIDS-compliant anonymized date inscans.tsv. However, it was only implemented in cases where the anonymized data started out in EDF format (i.e., onlymne_bids/copyfiles.pywas changed). In cases where the data start in another format, are anonymized, and then written to EDF(+), the result will be an invalid EDF file. This PR fixes that. As with #669, the BIDS-compliant anonymized date will still be written toscans.tsv; only the EDF file itself will be modified.The PR also does a small internal refactor to allow both EDF and BDF writing; this is needed for #1129, because EMG modality will have BDF(+) as a RECOMMENDED format.
Merge checklist
Maintainer, please confirm the following before merging.
If applicable: